Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(autocomplete): show dropdown on top only when there is room #11629

Merged
merged 1 commit into from
Feb 8, 2019
Merged

fix(autocomplete): show dropdown on top only when there is room #11629

merged 1 commit into from
Feb 8, 2019

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Feb 1, 2019 •
edited
Loading

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

The autocomplete options panel is shown using the top position even when there is no room to display the options. More details in #10859.

Issue Number:
Related to #11626. Related to #11575. Fixes #10859.

What is the new behavior?

This reverts commit 46e08e5 which reverted the previous fix (a37f7cc) to this issue due to a regression.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

At this time, this PR still causes the regression and no reproduction steps have been defined to allow us to investigate or fix the regression.

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Feb 1, 2019
@Splaktar Splaktar added this to the 1.1.13 milestone Feb 1, 2019
@Splaktar Splaktar added the P3: important Important issues that really should be fixed when possible. label Feb 1, 2019
@Splaktar Splaktar mentioned this pull request Feb 1, 2019
Closed
@Splaktar Splaktar force-pushed the autocomplete-panelPosition branch from 1656e0e to 0899e0e Compare February 1, 2019 19:04
@Splaktar
Copy link
Member Author

Splaktar commented Feb 1, 2019

OK, @marosoft here's the information that I have for when this new calculation doesn't work.

The use case is an md-autocomplete inside of an md-dialog. The expected behavior is that the options panel will position itself using bottom and this happened before this change went in.

autocomplete-before-new-calcuation

After this change to the calculation went in, the position is top, but the options panel is displayed below the bottom of the dialog (almost off screen).

autocomplete-options-below-dialog

@Splaktar
Copy link
Member Author

Splaktar commented Feb 5, 2019

I wasn't able to reproduce this in testing yesterday with md-autocomplete that uses md-floating-label.

Today I just learned that this issue was happening with md-chips that wrapped an md-autocomplete. I'll investigate that today.

@Splaktar
Copy link
Member Author

Splaktar commented Feb 5, 2019

Note that md-dropdown-position is not being set, so it's just defaulting to determining the dropdown placement based on available space in the viewport.

@Splaktar Splaktar added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Feb 5, 2019
@Splaktar
Copy link
Member Author

Splaktar commented Feb 5, 2019

The issue with the dropdown being displayed below the dialog has been tracked down. This was caused by custom CSS overrides conflicting with the updated positioning. The custom CSS can be adjusted.

The remaining issue is a concern that the behavior is changing where sometimes the dropdown is displayed on top where it used to be displayed on the bottom. There appears to be some assumption that bottom should be the default, whereas AngularJS Material has always had top as the default (if there is space). I'm going to check if the spec has any guidance on this, but it may be too disruptive to change the default at this time.

@Splaktar
Copy link
Member Author

Splaktar commented Feb 6, 2019 •
edited
Loading

The spec doesn't help us here. In an update that I think came in 2017, they moved away from popup/dropdown autocomplete design to an inline design:
material-spec-autocomplete

We've never had any requests (afaik) to move to that new design and we don't currently plan to do so.

Miles confirmed that md-autocompletion-position="bottom" solved the issue for the affected team that wanted them to always be positioned below the input.

@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Feb 6, 2019
@Splaktar Splaktar changed the title WIP: fix(autocomplete): show dropdown on top only when there is room fix(autocomplete): show dropdown on top only when there is room Feb 6, 2019
@Splaktar
Copy link
Member Author

Splaktar commented Feb 6, 2019

@jelbourn caretaker note: please check with Miles to verify that the affected team updated their app before syncing this to google3.

@jelbourn jelbourn merged commit 38fb991 into master Feb 8, 2019
@Splaktar Splaktar deleted the autocomplete-panelPosition branch February 8, 2019 23:31
@Splaktar Splaktar mentioned this pull request Mar 1, 2019
Closed
Splaktar added a commit that referenced this pull request Mar 10, 2019
@Splaktar
fix(autocomplete): default dropdown to position bottom as default
9fd7429 
Fixes #11656. Relates to #10859, #11629, #11575.
Splaktar added a commit that referenced this pull request Mar 10, 2019
@Splaktar
fix(autocomplete): default dropdown to position bottom as default
ea3b8c1 
fix horizontal alignment edge cases
fix $mdUtil.getViewportTop to handle when body scrolling is disabled

Fixes #11656. Relates to #10859, #11629, #11575.
@Splaktar Splaktar mentioned this pull request Mar 10, 2019
Merged
3 tasks
Splaktar added a commit that referenced this pull request Mar 10, 2019
@Splaktar
fix(autocomplete): default dropdown to position bottom as default
cab0dcf 
fix horizontal alignment edge cases
fix $mdUtil.getViewportTop to handle when body scrolling is disabled

Fixes #11656. Relates to #10859, #11629, #11575.
Splaktar added a commit that referenced this pull request Mar 18, 2019
@Splaktar
fix(autocomplete): default dropdown to position bottom as default
e41ef82 
fix horizontal alignment edge cases
fix $mdUtil.getViewportTop to handle when body scrolling is disabled

Fixes #11656. Relates to #10859, #11629, #11575.
andrewseguin pushed a commit that referenced this pull request Mar 20, 2019
@Splaktar @andrewseguin
fix(autocomplete): default dropdown to position bottom as default (#1…
7674959 
…1670)

fix horizontal alignment edge cases
fix $mdUtil.getViewportTop to handle when body scrolling is disabled

Fixes #11656. Relates to #10859, #11629, #11575.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@jelbourn jelbourn

@Splaktar Splaktar

@mmalerba mmalerba

Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P3: important Important issues that really should be fixed when possible. pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
Milestone
1.1.13
Development

Successfully merging this pull request may close these issues.

autocomplete: shows dropdown on top even when there is no room
4 participants
@Splaktar @jelbourn @googlebot @mmalerba